Skip to content
This repository has been archived by the owner on Feb 8, 2020. It is now read-only.

Add handling events by navigators not only on the top of the tree #2

Closed
wants to merge 15 commits into from
Closed

Add handling events by navigators not only on the top of the tree #2

wants to merge 15 commits into from

Conversation

osdnk
Copy link
Member

@osdnk osdnk commented Jul 14, 2019

No description provided.

satya164
satya164 reviewed Jul 15, 2019
View reviewed changes
Copy link
Member

@satya164 satya164 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'll be good to extract out the child listener stuff to a separate hook

src/NavigationContainer.tsx Outdated Show resolved Hide resolved
@osdnk
Copy link
Member Author

osdnk commented Jul 15, 2019

Could you take a look on tests pls? 😄 Sth's wrong

@satya164 satya164 force-pushed the @osdnk/dispatching-event-to-children branch 2 times, most recently from 044127a to 70f431c Compare July 16, 2019 10:55
@satya164 satya164 force-pushed the @osdnk/dispatching-event-to-children branch from 70f431c to 063c35a Compare July 16, 2019 10:59
@satya164
Copy link
Member

satya164 commented Jul 16, 2019 •
edited
Loading

I have rebased the PR against master.

Regarding tests, looks like a legit issue, no? The child state doesn't seem to be initialised properly in the global state.

I found another issue:

Kapture 2019-07-16 at 13 01 03

There's probably some stale state being updated somewhere. I think moving state to local will make it easier to handle.

satya164
satya164 reviewed Jul 16, 2019
View reviewed changes
src/useDescriptors.tsx Outdated Show resolved Hide resolved
src/useChildrenActionHelper.tsx Outdated Show resolved Hide resolved
src/useNavigationBuilder.tsx Outdated Show resolved Hide resolved
src/useOnAction.tsx Outdated Show resolved Hide resolved
src/useOnAction.tsx Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Jul 16, 2019 •
edited
Loading

Codecov Report

Merging #2 into master will decrease coverage by 2.21%.
The diff coverage is 94.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master       #2      +/-   ##
==========================================
- Coverage   95.16%   92.94%   -2.22%     
==========================================
  Files          11       13       +2     
  Lines         124      156      +32     
  Branches       25       33       +8     
==========================================
+ Hits          118      145      +27     
- Misses          5       10       +5     
  Partials        1        1
Impacted Files Coverage Δ
src/useNavigationHelpers.tsx 100% <ø> (ø) ⬆️
src/useDescriptors.tsx 100% <ø> (ø) ⬆️
src/NavigationContainer.tsx 100% <ø> (ø) ⬆️
src/NavigationBuilderContext.tsx 100% <ø> (ø) ⬆️
src/useChildActionListeners.tsx 100% <100%> (ø)
src/useNavigationBuilder.tsx 100% <100%> (ø) ⬆️
src/SceneView.tsx 80% <100%> (-14.74%) ⬇️
src/useOnChildUpdate.tsx 91.66% <91.66%> (ø)
src/useOnAction.tsx 95.45% <92.3%> (-4.55%) ⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ff3de1...c2fe6ee. Read the comment docs.

@satya164 satya164 force-pushed the @osdnk/dispatching-event-to-children branch from 07e6412 to 62b7128 Compare July 16, 2019 18:28
@satya164
Copy link
Member

I have fixed the tests and refactored the code a bit. Please take a look when you get time 😁

@satya164 satya164 force-pushed the @osdnk/dispatching-event-to-children branch from 62b7128 to e24469b Compare July 16, 2019 18:32
@satya164
Copy link
Member

Merged in 5eb12aa

@satya164 satya164 closed this Jul 17, 2019
@satya164 satya164 deleted the @osdnk/dispatching-event-to-children branch July 17, 2019 10:39
satya164 added a commit that referenced this pull request Aug 18, 2019
@satya164
feat: export individual navigators separately. fixes #2
873ace8
satya164 added a commit that referenced this pull request Aug 18, 2019
@satya164
feat: export individual navigators separately. fixes #2
65b0c46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@satya164 satya164 satya164 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@osdnk @satya164 @codecov-io